Module name derivation (PR1 of many) #58
Conversation
codetracer-python-recorder/resources/trace_filters/builtin_default.toml: codetracer-python-recorder/src/trace_filter/engine.rs: design-docs/adr/0013-reliable-module-name-derivation.md: design-docs/reliable-module-name-derivation-implementation-plan.md: design-docs/reliable-module-name-derivation-implementation-plan.status.md: docs/onboarding/trace-filters.md: Signed-off-by: Tzanko Matev <[email protected]>
Coverage SummaryRust (lines)
Python (statements)
Generated automatically via |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn resolve_module_name(&self, py: Python<'_>, absolute: &str) -> Option<String> { | ||
| if let Some(entry) = self.module_cache.get(absolute) { | ||
| return entry.value().clone(); | ||
| } | ||
| let resolved = module_name_from_roots(&self.module_roots, absolute) | ||
| .or_else(|| lookup_module_name(py, absolute)); | ||
| self.module_cache | ||
| .insert(absolute.to_string(), resolved.clone()); |
There was a problem hiding this comment.
The new resolve_module_name short-circuits to module_name_from_roots and caches the result without checking whether that dotted name is actually a syntactically valid module. When sys.path contains overlapping prefixes (e.g. /usr/lib/python3.11 before /usr/lib/python3.11/lib-dynload), the helper returns strings like lib-dynload._asyncio.cpython-311-x86_64-linux-gnu.so, which fail package selectors such as pkg:literal:_asyncio. Because the invalid name is cached, the lookup_module_name fallback is never invoked and those frames remain unmatchable. The resolver should validate the candidate with is_valid_module_name (or similar) and fall back to the sys.modules scan on failure before inserting it into the cache.
Useful? React with 👍 / 👎.
We are working on deriving a way to compute Python module names from the information that we get during tracing. This is first PR in a series of several